Skip to content

nvme: Add test for sending admin commands over disabled paths#178

Open
hreinecke wants to merge 1 commit intolinux-blktests:masterfrom
hreinecke:admin-cmds
Open

nvme: Add test for sending admin commands over disabled paths#178
hreinecke wants to merge 1 commit intolinux-blktests:masterfrom
hreinecke:admin-cmds

Conversation

@hreinecke
Copy link
Copy Markdown
Contributor

ANA states only apply to commands to individual namespaces, not to commands for the controller. So add a testcase to check if we can send admin commands over inaccessible paths.

@hreinecke hreinecke force-pushed the admin-cmds branch 2 times, most recently from 8c6224a to 05fbdf9 Compare May 30, 2025 12:28
ANA states only apply to commands to individual namespaces, not to
commands for the controller. So add a testcase to check if we can
send admin commands over inaccessible paths.

Signed-off-by: Hannes Reinecke <[email protected]>
Comment thread tests/nvme/064
ns=$(_find_nvme_ns "${def_subsys_uuid}")
[ -z "${ns}" ] && return 1
nvme nvm-id-ctrl "/dev/${ns}"
nvme nvm-id-ns "/dev/${ns}"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everytime nvm-id-ctrl or nvm-id-ns is touched this test will break. I think it would be better to test for expected values.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran this test case, and observed the failure message below:

nvme/064 (tr=loop) (test admin commands on disabled paths)   [failed]
    runtime  0.450s  ...  1.114s
    --- tests/nvme/064.out      2025-06-02 20:38:46.278859422 +0900
    +++ /home/shin/Blktests/blktests/results/nodev_tr_loop/nvme/064.out.bad     2025-06-05 15:23:54.194590517 +0900
    @@ -6,16 +6,22 @@
     dmrl   : 0
     dmrsl  : 0
     dmsl   : 0
    +kpiocap: 0
    +wzdsl  : 0
    +aocs   : 0
    +ver    : 0x0
    ...
    (Run 'diff -u tests/nvme/064.out /home/shin/Blktests/blktests/results/nodev_tr_loop/nvme/064.out.bad' to see the entire diff)

It looks that the new fields that nvme-id-ctrl and nvme-idns commands report are causing the failure.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make this test a bit future proof, it really should check for specific fields, as we do in nvme/059:

 nvme_awupf=$(nvme id-ctrl /dev/"${ctrl_dev}" | grep awupf | awk '{ print $3}')

BTW, I still think we should start using the json output for nvme-cli together with jq, e.g:

nvme id-ctrl /dev/nvme0 --output json | jq .awupf

kawasaki pushed a commit to kawasaki/blktests that referenced this pull request Mar 20, 2026
ANA states only apply to commands to individual namespaces, not to
commands for the controller. So add a testcase to check if we can
send admin commands over inaccessible paths.

Linke: linux-blktests#178
Signed-off-by: Hannes Reinecke <[email protected]>
[Shin'ichiro: renumbered test case and aligned file mode]
[Shin'ichiro: modified to check only exit codes of nvme commands]
Signed-off-by: Shin'ichiro Kawasaki <[email protected]>
@kawasaki
Copy link
Copy Markdown
Collaborator

Today, I found a time slot to take a look in this PR. I renumbered the test case and ran it with the recent kernel and nvme-cli, I noticed that nvm-id-ctrl and nvm-id-ns commands just failed with inaccesible ANA , and printed the error messages below:

nvm identify controller: Resource temporarily unavailable
get-namespace-id: Resource temporarily unavailable

So it does not look meaningful to check output of these commands.

How about to redirect the command outputs to FULL file for logging, and just check the exit status of the nvme commands? Based on this idea, I modified the test script (link).

@hreinecke
Copy link
Copy Markdown
Contributor Author

In principle, yes. But the patch fixing that had been ... well, not directly ignored, but also not applied. On the grounds that there is no use-case. Maybe we can bring it up at LSF (at the blktest session?) to see how we can or should progress here.

@kawasaki
Copy link
Copy Markdown
Collaborator

In principle, yes. But the patch fixing that had been ... well, not directly ignored, but also not applied. On the grounds that there is no use-case. Maybe we can bring it up at LSF (at the blktest session?) to see how we can or should progress here.

Okay, sounds like a plan. Let's revisit this after the talk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants